If scripting has to set it in anycase why not use scripting for the whole thing? You can receive and send MAVLink messages from Lua.

@IamPete1:

If scripting has to set it in anycase why not use scripting for the whole thing? You can receive and send MAVLink messages from Lua.

Two reasons:

1. At least for the CAMERA_INFORMATION message, the AP_Camera driver is already sending this message. I didn't want a situation where the AP_Camera driver is sending the message and the scripting is sending a conflicting message.
2. These messages are requested rather than streamed. Is there a nice way to handle this in a script, rather than just periodically sending the message?

@Hwurzburg:

definitely needs to be disabled in minimize include and build_options/extract_featues files added

Done.

@Hwurzburg:

needs squash and library split

What do you mean?

• the commits are already split per library.
• the commits are logical changes that add one thing at a time, rather than a number of unrelated changes in a single commit.

2. These messages are requested rather than streamed. Is there a nice way to handle this in a script, rather than just periodically sending the message?

There is a way to do this, I actually did exactly that and was about to open a PR for it, but I also was thinking about redoing it to something like your approach here. It's easy enough to intercept a MAV_CMD_REQUEST_VIDEO_STREAM_INFORMATION, but that's being deprecated in favor of MAV_CMD_REQUEST_MESSAGE. The issue with the latter is that only one message request is cached for the lua script to grab, so I had to run the script at high speed to make sure I didn't miss a message request if the GCS sent a bunch of them all at once.

So, I think there are some advantages to your approach.

My lua script for reference
video_stream.zip

only one commit per library is allowed in final pr

squash to one commit then run Tools/gittools/git-subsystems-split script and push back

only one commit per library is allowed in final pr

This is definitely not true and IMO reduces clarity.

@Hwurzburg, @peterbarker: Squashed.

@peterbarker: Is FenceFloorEnabledLanding a flapping test? No code change, but it's now failing?

Its a new test - I haven't seen it flap

I can't trigger tests, so I've rebased on master again to trigger a rebuild.

Also the AP_CAMERA_INFO_FROM_SCRIPT_ENABLED AP_MAVLINK_MSG_VIDEO_STREAM_INFORMATION_ENABLED might need to be in the scripting bindings too? You can do a depends on methods too if you want the main scripting support.

@IamPete1: I've updated the Lua bindings for the new structs and methods to depend on AP_CAMERA_INFO_FROM_SCRIPT_ENABLED.

Rebased on master to get the changes from #28128 so CI passes.

Binary Name      Text [B]         Data [B]     BSS (B)        Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  ---------------  -----------  -------------  ----------------------------  -------------------------
antennatracker   3512 (+0.2622%)  0 (0.0000%)  0 (0.0000%)    3512 (+0.2618%)                                  620996
arducopter       3428 (+0.1896%)  0 (0.0000%)  4 (+0.0015%)   3428 (+0.1892%)                                  150640
ardusub          3428 (+0.2142%)  0 (0.0000%)  4 (+0.0015%)   3428 (+0.2139%)                                  359756
blimp            3512 (+0.2579%)  0 (0.0000%)  0 (0.0000%)    3512 (+0.2575%)                                  598928
arduplane        3428 (+0.1899%)  0 (0.0000%)  4 (+0.0015%)   3428 (+0.1895%)                                  153320
ardurover        3432 (+0.2070%)  0 (0.0000%)  0 (0.0000%)    3432 (+0.2067%)                                  302104
arducopter-heli  3428 (+0.1897%)  0 (0.0000%)  -4 (-0.0015%)  3428 (+0.1893%)                                  151600

It's still big, but if the ifdefs work now, it should not be a problem.

Running locally, I get the following:

This branch, AP_CAMERA_INFO_FROM_SCRIPT_ENABLED = 0:

Build directory: /ardupilot/build/sitl
Target          Text (B)  Data (B)  BSS (B)  Total Flash Used (B)  Free Flash (B)  External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter   4172858    191517   222752               4364375  Not Applicable  Not Applicable

master branch:

Build directory: /ardupilot/build/sitl
Target          Text (B)  Data (B)  BSS (B)  Total Flash Used (B)  Free Flash (B)  External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter   4172978    191453   222752               4364431  Not Applicable  Not Applicable  

Deltas between disabled and master:

Target          Text (B)  Data (B)  BSS (B)  Total Flash Used (B)  Free Flash (B)  External Flash Used (B)
----------------------------------------------------------------------------------------------------------
bin/arducopter      -120        64        0                   -56  Not Applicable  Not Applicable  

only one commit per library is allowed in final pr

This is definitely not true and IMO reduces clarity.

then we definitely need a change to developer WIKI instructions... @peterbarker ?:
"The commits of the change should be squashed (see Interactive Rebase: cleaning up commits) into one commit and then the “Tools/gittools/git-subsystems-split” script run to create a single commit for each library module affected.

@Hwurzburg, I think the requirement is that individual commits should not affect more than one subsystem but multiple commits affecting a single subsystem is OK. The git-subsystems-split script is probably meant to divide up a single commit but not to consolidate commits.